fix: properly surface Treezor user rejection errors#8490
Open
Olivier-BB wants to merge 11 commits into
Open
Conversation
Contributor
|
@copilot resolve the merge conflicts in this pull request |
…ve-error-trezor # Conflicts: # packages/keyring-controller/package.json # packages/transaction-controller/src/TransactionController.ts Co-authored-by: dawnseeker8 <7315988+dawnseeker8@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MetaMask/core/sessions/64a5b953-7f1d-4e47-be62-0f18a2f761f9 Co-authored-by: dawnseeker8 <7315988+dawnseeker8@users.noreply.github.com>
Resolved the merge conflicts in commit
The yarn.lock was updated accordingly, and all 1,400+ tests across both affected packages continue to pass. |
Contributor
|
@Olivier-BB please update this branch and fix CI so we can get this reviewed and merged. |
Contributor
|
@copilot resolve the merge conflicts in this pull request |
…essage methods - Introduced tests to ensure that errors with code 4001 and string rejections are normalized to user rejection errors. - Added checks to verify that non-rejection errors are re-thrown unchanged, including those with circular references. - Included a test for normalizing cancellation-like errors in signPersonalMessage to a 4001 code.
- Consolidated multiple lines of jest.spyOn calls into single lines for improved readability. - Maintained existing test functionality while enhancing code clarity.
Document the user-rejection error normalization changes in keyring-controller and transaction-controller, satisfying the changelog check for PR #8490. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
@copilot resolve the merge conflicts in this pull request |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Treezor user rejection errors were not being surfaced clearly, which could result in uninformative error handling around rejected requests.
This change updates the relevant controller flows so Treezor user rejection errors are surfaced properly, and adds test coverage in both
keyring-controllerandtransaction-controllerto verify the behavior.KeyringControllernow catchessignMessage/signPersonalMessage/signTransactionerrors (and conditionallysignTypedMessage) and normalizes cancellation-like errors by recursively inspectingcode,message,stack,cause, andoriginalError.TransactionControllersimilarly detects cancellation-like errors (including a Trezor/"unknown error" string case), persists them asuserRejectedRequest, and updates tests to assert the persisted errorcode.The
TransactionControllertests for "cancelled"/"canceled" signing error normalization have been updated to use theKeyringController:signTransactionmessenger action handler (rather than the legacysignoption) to properly exercise the error path, reflecting the current signing architecture.References
Checklist
Note
Medium Risk
Changes error shapes thrown from core signing and transaction-failure paths; consumers that matched on raw hardware error text or KeyringControllerError wrapping may see different errors, though behavior for genuine user rejections is more correct.
Overview
Hardware wallets (e.g. Trezor) often surface user cancellations as vague strings or wrapped errors instead of EIP-1193
userRejectedRequest(4001). This PR aligns signing and transaction failure paths so UIs and downstream code can treat rejections consistently.KeyringControlleradds@metamask/rpc-errorsand wrapssignMessage,signPersonalMessage,signTransaction, and (when applicable)signTypedMessageso cancellation-like failures becomeproviderErrors.userRejectedRequestwith the standard MetaMask denial message, preserving optionaldatawhen present. Detection recurses throughcode,message,stack,cause, andoriginalError, including4001, cancel/canceled strings, andfailure_actioncancelled.TransactionControlleradds parallel#hasUserRejectedMessagelogic (including a Trezor “unknown error” string heuristic) and, in#failTransaction, persists and publishes the normalized rejection error on failed txs andtransactionFailedevents instead of opaque hardware errors.Changelogs and broad test coverage document the new behavior; transaction signing tests mock
KeyringController:signTransactionto exercise the path.Reviewed by Cursor Bugbot for commit fc5e685. Bugbot is set up for automated code reviews on this repo. Configure here.